feat(service): scope-explicit install/uninstall/start/stop (--scope) - #123
feat(service): scope-explicit install/uninstall/start/stop (--scope)#123MichaelTaylor3d wants to merge 5 commits into
Conversation
Scope-explicit service verbs so an elevated installer can register a system-level, reboot-surviving dig-node service. Co-Authored-By: Claude <noreply@anthropic.com>
Retire the unconditional `PREFERS_USER_LEVEL` decision const and thread a runtime ServiceScope through the service verbs, so an elevated installer can register a system-level (boot-started) dig-node service on a headless host while an unelevated desktop install keeps today's user-level behaviour. * resolve_scope(choice, os_supports_user, is_root) is the ONE scope decision and is PURE: every input is a parameter, so the complete 12-row decision table is asserted on any host at any privilege level. * install clears any registration at the OTHER scope BEFORE creating at the requested one, so a host upgrading from a user-level install never runs two units racing for the node's port. * uninstall --scope auto sweeps both scopes and reports per scope; anything short of a complete removal is an error, never a silent success. * The §565 privileged-target gate now genuinely fires on unix system scope, so its refusal names the offending path LEVEL instead of just the program path. Refs #526. Co-Authored-By: Claude <noreply@anthropic.com>
…ENT_LOG Bump to 0.69.0 (minor: a new backwards-compatible --scope flag). Co-Authored-By: Claude <noreply@anthropic.com>
…to default A lib-level scope test cannot see a verb that forgot to accept the flag, so assert it against the BUILT binary's own help (and that an unknown value is a usage error rather than a silent fallback to the default). Co-Authored-By: Claude <noreply@anthropic.com>
4d19fe1 to
a4eaa67
Compare
…dvisory The macOS service-smoke leg caught this: launchctl print gui/<uid>/<label> cannot see a per-user agent from a session with no GUI domain, so a probe-gated uninstall reported "no service registration was found" and exited 6 on a service that WAS registered. The OS deregistration call is the authority, not the probe. A RemovalMode now distinguishes the scope the operator NAMED (removed unconditionally, which is also the pre-#526 behaviour) from a scope merely being SWEPT for a stale registration (probe-gated, so a scope nobody asked about is never written to). A scope whose absence was never established is reported as indeterminate rather than as a clean uninstall. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED (fresh-context correctness gate, head 7c93cd9bc17c790d47836f198fb35d77ef3b3668).
What I verified and accept:
- Reboot survival is a real mechanism, not a flag.
plan.autostart = truereachesservice-manager0.7.1, whose systemd backend writes to the GLOBAL unit dir withWantedBy=multi-user.target(systemd.rs:320) and runssystemctl enable(systemd.rs:154, creating themulti-user.target.wantssymlink); launchd writes/Library/LaunchDaemons/<label>.plistwithRunAtLoad=true(launchd.rs:256);scsetsstart=Auto(sc.rs:191). Not a written-but-never-enabled unit. - Backwards compatibility.
ScopeChoice::Autois the derivedDefaultand the clap default,resolve_scope(Auto, true, false) == User, and no existing default moved. An older dig-installer passing no flag is unchanged;start/stopwere already user-level pre-PR. --scope systemwithout root isPermissionDenied, loud and actionable, never downgraded. To the harder question: it creates no new hard-failure class, because pre-PR there was no way to ASK for system scope andAuto-unprivileged still resolves toUser. Hard refusal is the right call here.- The
securityrefactor is semantics-preserving.find(|c| !p(c)).is_none()is exactlyall(p);self_healandtlssee identical behaviour. - Test falsifiability: the 12-row
resolve_scopetable is notcfg-gated, no decision logic landed insrc/bin(dign.rsis a 3-linemain), the previously-vacuouscontains(offending_path)is now an exact-phrase assertion,a_swept_scope_is_never_written_to...asserts the exact call transcript, and no test requires elevation. - All 20 check-runs are
successfor this head SHA (verified via the check-runs API, notgh pr checksalone), and there are zero open review threads.
Two findings block; three are follow-ups. Details inline.
Finding 3 (follow-up; no diff line to anchor an inline thread to)
.github/workflows/service-smoke.yml is untouched by this PR, so its CI evidence covers only the paths that already worked.
Follow-up - does not block this merge, but it blocks CLOSING dig_ecosystem#526. This job is unchanged by the PR and runs unelevated, so on ubuntu/macOS it exercises auto => user scope only; the Windows leg is SCM, which never had the bug. The new unix system-scope path therefore has no real-OS evidence anywhere, so the PR body’s "the only evidence that the new scope path works against actual OS service managers" overstates what the three legs prove. The mechanism is correct by inspection (see the review summary), but the #526 acceptance bar is "registered at a scope that ACTUALLY survives a reboot".
Cheap to close in this same job: the runner already has passwordless sudo (line 70) and already exports DIG_NODE_ALLOW_INSECURE_SERVICE_TARGET. Add an ubuntu step - sudo -E dig-node install --scope system --json, then assert /etc/systemd/system/dignetwork-dig-node.service exists, systemctl is-enabled says enabled, and /etc/systemd/system/multi-user.target.wants/dignetwork-dig-node.service is a symlink; then sudo -E dig-node uninstall --scope system and assert all three are gone. That asserts ENABLEMENT, not merely a written unit file, and needs no elevation from cargo test.
| // "there was nothing here" case (the OS says so twice) — keep the reason for context, | ||
| // but only a scope we DID see, or could not read at all, is unresolved. | ||
| removal.indeterminate = probe.is_err(); | ||
| removal.error = Some(e.to_string()); |
There was a problem hiding this comment.
BLOCKING (1/2) - a Requested-scope removal that FAILS is reported as "nothing to uninstall", which is the defect class this PR was written to fix, reached from the other side of the branch.
You correctly declared the probe advisory. But the delete-failure arm re-trusts it: indeterminate = probe.is_err(), and the real launchd/systemd probe never returns Err (query_installed collapses every failure into false - see my other blocking comment), so indeterminate is always false here. uninstall_outcome then sees removed.is_empty() with no problems and returns ErrorKind::NotFound - "no service registration ... was found - nothing to uninstall", exit 6.
Concrete failure: macOS, ssh session with no Aqua domain, a per-user agent IS registered. launchctl print gui/<uid>/<label> false-negatives (found=false - exactly the case your regression test names). delete is launchctl unload then remove_file; if unload fails in that domain, service-manager returns Err before removing the plist. Result: the operator is told nothing was installed, while the plist remains and starts the agent at their next login. That is #526's "uninstall leaves nothing behind" violated with a confident claim of absence.
The in-code rationale - "the OS says so twice" - does not hold in Requested mode, because one of the two says is the signal you just declared untrustworthy. The fix needs a discriminator that is not the probe: classify by the delete error (NotFound-class => genuinely nothing here => NOT_FOUND; PermissionDenied/other => indeterminate). Your own mock already encodes that distinction (failing_delete returns PermissionDenied "Failed to connect to bus"), so this is testable with the fixtures already present.
| let unit = format!("{service_name}.service"); | ||
| let mut cmd = std::process::Command::new("systemctl"); | ||
| if PREFERS_USER_LEVEL { | ||
| if scope.is_user() { |
There was a problem hiding this comment.
BLOCKING (2/2) - query_installed is a bool that cannot say "I could not check", which silently voids the cross-scope migration contract SPEC.md:1691 states as a MUST.
.map(|s| s.success()).unwrap_or(false) makes three states indistinguishable: (a) no unit exists, (b) systemctl --user could not reach a bus, (c) systemctl is missing. As root, systemctl --user cat dignetwork-dig-node.service exits non-zero with Failed to connect to bus - the very condition this PR exists to work around - and is reported as "no registration at user scope". Same on macOS for launchctl print gui/<uid>/.... Three silent consequences:
installmigration (service.rs:535,Sweptmode). The scenario SPEC.md:1691 names verbatim - "a host upgrading from a previous user-level install" - is exactly the one it cannot handle: root can neither see nor remove a DIFFERENT user’s systemd user unit.found=false,indeterminate=false, somigrated_from_scopereports a clean nothing-there and the human summary prints no WARN. After an elevated dig-installer run the desktop user’s stale user unit still starts at their login and races the system unit for port 9778 - precisely the double-bind the migration claims to prevent. Relocating the registration closed nothing, because the old location still gets its chance.uninstall --scope autoas root sweeps[System, User]; the user leg collapses to "nothing there" and the command reportsremoved_scopes:["system"]as a clean success while a per-user registration may remain.ScopeRemoval::indeterminatefrom a probe failure is unreachable in production. No real backend’sis_installedcan returnErr(only a failingSystemServiceBackend::newinsideuninstallreaches that state), so the three-state guarantee in SPEC.md:1706 is proven only by a mock the real backend cannot emulate.
Minimum to unblock (either, both cheap): make the probe tri-state - have the real probe distinguish "could not determine" from "absent" (classify the tool stderr, or return Err when the tool ran but the failure is not unit-not-found) so indeterminate becomes reachable; and/or scope the SPEC clauses to what the code can actually do (same-principal registrations) and emit an explicit note on a root system-scope install that per-user registrations owned by other users are neither visible nor removable, naming the --scope user command to run as that user. What must not stand is a SPEC MUST that the implementation silently cannot meet.
What changed
dig-node installwas hardcoded to prefer a user-level registration on every non-Windowsplatform (
const PREFERS_USER_LEVEL: bool = true). Run undersudo dig-installer, that tries asystemd --userunit — and root has no systemd--userD-Bus session(
Failed to connect to bus: Operation not permitted, observed asexited with 6), so an elevatedheadless install could not get a reboot-surviving registration at all. dig-node's own native
packages already register SYSTEM scope (
packaging/linux/systemd/net.dignetwork.dig-node.serviceis
WantedBy=multi-user.targetas root;packaging/macos/scripts/postinstallbootstrapslaunchctl … system), so the CLI verb was the outlier. This makes the CLI coherent with thepackages, without changing the no-root desktop UX.
The compile-time constant is retired and replaced by a runtime scope threaded through the verbs:
CLI surface (identical on
dig-nodeand thedignalias):resolve_scopeis the ONE scope decision and it is PURE — nocfg!, nogeteuid, nofilesystem.
auto= system when root, user otherwise; an explicit--scopeis AUTHORITATIVE andnever silently overridden; an OS with no user domain (Windows SCM) always resolves to system.
install— the other scope is probed and, if it holds aregistration, deregistered BEFORE creating at the requested scope. Without it, a host upgrading
from a user-level install ends up with two units both binding the node port and the stale one can
win. Best-effort and reported (
result.migrated_from_scope), never fatal.uninstall --scope autosweeps BOTH scopes and reports per scope (result.removed_scopes); ascope found-but-not-removed, or one left indeterminate, is an ERROR naming the scope — never a
silent success.
RemovalMode). Thescope the operator NAMED is deregistered unconditionally — as
uninstallalways did pre-#526 —because
launchctl print gui/<uid>/<label>cannot see a per-user agent from a session with no GUIdomain and false-negatives on a service that IS registered. A merely-SWEPT scope stays
probe-gated, so a scope nobody asked about is never written to. The macOS
service-smokejobcaught this for real (first push:
no service registration … was found, exit 6, on a servicethe same job had just installed); the fix has a named regression test and the leg is now green.
--scope systemis refused loudly, never downgraded to user scope (a silentdowngrade produces exactly the registration that does not survive a reboot).
refusal now names the offending path level (
security::first_unprivileged_ancestor) insteadof only the program path, so an operator can act on it. The dig-installer's root-owned
/opt/dig/binclears the gate.Backwards compatibility
--scopedefaults toauto, andautounprivileged on a user-capable OS resolves toUser—byte-for-byte the pre-change behaviour. An older dig-installer that passes no flag keeps working
(asserted by
scope_choice_default_is_auto_so_an_older_installer_passing_no_flag_is_unchanged).No public item was removed except the private
PREFERS_USER_LEVELconst.Blast radius checked
gitnexus is disabled in the loop (CLAUDE.md §2.0 temp override), so the radius was taken with
repo-wide ripgrep + direct reads:
PREFERS_USER_LEVEL(removed)service.rs(SystemServiceBackend::new,query_installed,launchd_domain_target)SystemServiceBackend::newservice.rsquery_installed,launchd_domain_targetservice.rsonlyensure_service_target_is_safeservice.rsonlyservice::{install,uninstall,start,stop}entrypoint.rsdispatch ONLY (repo-wide grep)security::dir_is_privilegedfirst_unprivileged_ancestorCross-repo consumers (dig-installer, the native packages) drive the CLI, not the Rust API, and
the flag is optional with an unchanged default — so nothing outside this repo must change to keep
working. Risk assessed MEDIUM: privileged-service registration is custody-adjacent (the §565 LPE
gate sits on this path), but the gate got STRICTER on unix, not looser, and no existing default
moved.
Test evidence (real output)
Every PR check is GREEN, including all three
service-smokelegs — a REALinstall/start/reinstall/uninstall round-trip against real
systemctl/launchctl/sc.exeonubuntu-latest,macos-14andwindows-latest, which is the only evidence that the new scope pathworks against actual OS service managers.
cargo test -p dig-node-service— 264 unit + 101 integration, all green:The 12-row decision table (the primary evidence — deliberately NOT
#[cfg(unix)]-gated, since acfg-gated scope test is unfalsifiable on a Windows host):
Each new test proved load-bearing by mutation (committed first, then reverting only the fix):
Autoprivilege branchesresolve_scope_decision_table…,scope_choice_default_is_auto…reinstall(the placement bug)install_deregisters_the_other_scope_before_creating_at_the_requested_oneuninstall_fails_loudly_when_a_found_registration_could_not_be_removed,uninstall_reports_an_indeterminate_scope…a_system_scope_registration_requires_root_on_a_user_capable_osa_refused_system_target_names_the_offending_directory_level--scopefrom thestopverbevery_service_verb_accepts_the_scope_flag_defaulting_to_autoa_requested_scope_is_removed_even_when_the_probe_cannot_see_ita_swept_scope_is_never_written_to_on_a_probe_that_sees_nothing,install_touches_nothing_at_the_other_scope_when_it_holds_no_registrationA false green was caught and fixed during that pass:
contains(offending_path)is VACUOUS, becauseevery ancestor is a string prefix of the program path the message already prints — the test now
asserts the exact naming phrase instead.
Coverage (
cargo llvm-cov -p dig-node-service --lib --summary-only, the same command on bothsides) — the new code RAISES it:
service.rsregionsservice.rslinesorigin/main(This repo's CI measures coverage across all targets and is measure-only; the lib-only figures above
are directly comparable to each other.)
cargo fmt --all -- --checkclean;cargo clippy -p dig-node-service --all-targets --all-features -- -D warningsclean.Docs kept coherent in the same unit
SPEC.md§9.1 rewritten as the normative scope model (the per-scope on-disk locations, what makesa registration survive a reboot, the resolution rules, the migration + sweep contracts, the
--scopedefault), §9.2c updated for the path-level refusal, §8.1 lists the flag.DEVELOPMENT_LOG.md— a durable entry on root having no systemd user bus undersudo.README.md— the operator-facing scope/--scopeexplanation.Version
0.69.0 (minor — a new backwards-compatible capability;
feat).Cargo.lockre-synced in thesame commit. No
package.jsonin this repo.NOT proven here
A real headless-Linux
sudo dig-installerrun: there is no Linux host in this lane, and anelevation-dependent test would give every other lane a false RED on an unelevated dev box
(dig_ecosystem#1865). Every scope decision is proven host-independently instead; the end-to-end
elevated install stays dig_ecosystem#526's own acceptance step.
Refs dig_ecosystem#526.